fix(sessions): Surface failed agent prompt sends to the user#2689
Conversation
|
React Doctor found no issues in the changed files. 🎉 Reviewed by React Doctor for commit |
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
packages/ui/src/features/sessions/sendPromptToAgent.test.ts:36-58
The two rejection tests share identical structure (mock rejection → call → waitFor toast assertion) and differ only in the thrown value and expected message. Per the team's preference for parameterised tests, they can be collapsed into a single `it.each`, reducing duplication without losing coverage.
```suggestion
it.each([
[new Error("Agent server is not reachable"), "Agent server is not reachable"],
["boom", "Failed to send your message to the agent. Please try again."],
])(
"surfaces rejection %s as an error toast with message %s",
async (rejection, expectedMessage) => {
mockSender.mockRejectedValueOnce(rejection);
sendPromptToAgent("task-1", "hello");
await vi.waitFor(() =>
expect(toast.error).toHaveBeenCalledWith(expectedMessage),
);
},
);
```
Reviews (1): Last reviewed commit: "surface failed agent prompt sends to the..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
charlesvien
added
the
Stamphog
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
No showstoppers. The fix correctly surfaces silently-swallowed send failures via toast, the type contract change is consistent across all usages, and the new tests cover the added error paths. The bot comment is a style suggestion about parameterised tests, not a substantive concern.
github-actions
Bot
dismissed
their stale review
New commits pushed (delta classified non_trivial_delta) — stamphog approval dismissed; re-review running automatically.
There was a problem hiding this comment.
Solid bug fix that surfaces silently-swallowed send errors via toast. The type contract change is consistent (one implementation, correctly updated), the catch handler is appropriate, and the new tests already use parameterised form addressing the bot's style suggestion.
1 participant
Problem
Button, review and skill prompts route through
sendPromptToAgent, which fired the send asvoid(fire and forget). A rejected send (auth failure, sandbox unreachable, agent process died, dropped clouduser_message) was swallowed, so the chat showed "Generated in Xs" with no reply and no error. This is one of the paths behind reports of the agent randomly not responding on local and cloud tasks. The chat-input path already toasted these errors; the button/review/skill paths did not.Changes
AgentPromptSenderport returnsPromise<void>instead ofvoidsendPromptToAgentcatches rejections and showstoast.errorwith a generic fallbacksendPromptso the rejection reaches the catchsendPromptToAgent.test.tscovering reject-with-Error, non-Error fallback and no-toast-on-successCovered callers: CommentAnnotation, PendingReviewBar, PrCommentThread, useFixWithAgent, SkillButtonsMenu. Both local and cloud sessions throw on a failed send, so both are now surfaced.
How did you test this?
vitest runonsendPromptToAgent.test.ts(3/3 pass)vitest runonpackages/ui/src/features/sessions(225/225 pass, no regression)biome lintandbiome formatclean on the changed files@posthog/quilldist errors are unrelated)Automatic notifications